dispatch Series[datetime64] ops to DatetimeIndex #19024
Conversation
|
rebase and push again, fixed some hanging by Travis CI |
| @@ -744,6 +744,13 @@ def wrapper(left, right, name=name, na_op=na_op): | |||
| return NotImplemented | |||
|
|
|||
| left, right = _align_method_SERIES(left, right) | |||
| if is_datetime64_dtype(left) or is_datetime64tz_dtype(left): | |||
There was a problem hiding this comment.
and until / unless you want to limit / remove _TimeOp (which is actually ok with me). then this doesn't belong here as I have commented before.
You are welcome to put it in _TimeOp for now or rip out TimeOp.
|
I think it will be much cleaner / better to have 1 PR which addresses any/all of TimeOp issues. You seem to want to change it, which is totally fine. But rather than just hack around, let's blow it away. |
Codecov Report
@@ Coverage Diff @@
## master #19024 +/- ##
==========================================
+ Coverage 91.56% 91.58% +0.01%
==========================================
Files 150 150
Lines 48942 48897 -45
==========================================
- Hits 44816 44784 -32
+ Misses 4126 4113 -13
Continue to review full report at Codecov.
|
|
Yes, I very much want to get rid of
For reasons discussed elsewhere, putting it in |
|
that would ls be fine |
doc/source/whatsnew/v0.23.0.txt
Outdated
| @@ -208,6 +208,9 @@ Other API Changes | |||
| - In :func:`read_excel`, the ``comment`` argument is now exposed as a named parameter (:issue:`18735`) | |||
| - Rearranged the order of keyword arguments in :func:`read_excel()` to align with :func:`read_csv()` (:issue:`16672`) | |||
| - The options ``html.border`` and ``mode.use_inf_as_null`` were deprecated in prior versions, these will now show ``FutureWarning`` rather than a ``DeprecationWarning`` (:issue:`19003`) | |||
| - Subtracting ``NaT`` from a :class:`Series` with ``dtype='datetime64[ns]'`` returns a ``Series`` with ``dtype='timedelta64[ns]'`` instead of ``dtype='datetime64[ns]'``(:issue:`18808`) | |||
| - Operations between a :class:`Series` with dtype ``dtype='datetime64[ns]'`` and a :class:`PeriodIndex` will correctly raises ``TypeError`` (:issue:`18850`) | |||
| - Subtraction of :class:`Series` with timezone-aware ``dtype='datetime64[ns]'`` will mis-matched timezones will raise ``TypeError`` instead of ``ValueError`` (issue:`18817`) dtypewith mis-matched timezones will now raise a ``TypeError`` instead of a ``ValueError`` (:issue:`18817`) | |||
There was a problem hiding this comment.
looks like some text got duped in the last entry
| @@ -107,7 +107,7 @@ def test_shift(self): | |||
| # incompat tz | |||
| s2 = Series(date_range('2000-01-01 09:00:00', periods=5, | |||
| tz='CET'), name='foo') | |||
| pytest.raises(ValueError, lambda: s - s2) | |||
| pytest.raises(TypeError, lambda: s - s2) | |||
There was a problem hiding this comment.
is this in the whatsnew notes API changed section?
|
see how easy that was! removing code is fun! anyhow obviously want to come back and clean this up even more, and definitly add some comments on what is non-obvious in the code (e.g. where datetimes are dispatched to DTI) |
This is the culmination of a bunch of recent work.
git diff upstream/master -u -- "*.py" | flake8 --diffIf merged, this will subsume #18960 and will obviate parts of #18964. This will also fix (but not test, so we'll leave the issue open for now) the lack of overflow checks #12534. Also in a follow-up we'll be able to remove a bunch of _TimeOp.